Pqc falcon512#32
Conversation
…tron into pqc-falcon512 # Conflicts: # actuator/src/main/java/org/tron/core/actuator/VMActuator.java
Mirrors block_fetch_latency for transactions: measures the end-to-end round-trip from sending GET_DATA to receiving the full TXS message, using the timestamp already stored in PeerConnection.advInvRequest at fetch-dispatch time. Records nothing for transactions pushed via gossip (no prior GET_DATA), which is intentional — this metric only captures the active-fetch path. Overhead is ~500 ns per tx (Item allocation + ConcurrentHashMap.remove + currentTimeMillis + histogram observe), negligible even at >1k TPS. Useful for PQ migration baseline / stress-test comparison: shows how much extra time the bigger Falcon-512 payloads add to in-flight transaction propagation, independent of local processing cost. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
1 issue found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…ransaction The original placement in handleTransaction() was dead code: processMessage() drains advInvRequest at L92-95 (before enqueueing tx work onto the smartContract/queue handlers), so by the time the worker thread reaches handleTransaction() the timestamp is already gone and remove(item) always returns null. Move the histogram observe into the same draining loop in processMessage(), using a single currentTimeMillis() reference captured just before the loop. This is both correct (we observe at the only remove() call that ever sees a non-null value) and slightly cheaper (one currentTimeMillis() per TRXS message instead of per tx). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
What does this PR do?
Why are these changes required?
This PR has been tested by:
Follow up
Extra details
Summary by cubic
Adds a Prometheus histogram
tron:tx_fetch_latency_secondsto measure GET_DATA → TXS round-trip for actively fetched transactions, as a companion totron:block_fetch_latency_seconds. Observed in theprocessMessagedrain loop so samples are recorded, with one clock read per TXS message.TX_FETCH_LATENCYinMetricKeysand initialized it inMetricsHistogram.TransactionsMsgHandler, observes latency viaPeerConnection.advInvRequest; ignores gossip-pushed TXs.Written for commit 9cb41b9. Summary will update on new commits. Review in cubic